Translate URL track-index state through profile sanitization#6000
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6000 +/- ##
==========================================
+ Coverage 83.77% 83.81% +0.03%
==========================================
Files 329 329
Lines 34423 34554 +131
Branches 9627 9661 +34
==========================================
+ Hits 28839 28960 +121
- Misses 5155 5165 +10
Partials 429 429 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Thanks for the PR, it looks like it's working mostly well! I found one more case where publishing makes some tracks reappear again. For example use this deploy preview:
Then click "re-upload", uncheck "Include hidden threads" and then upload it. Notice that firefox.com content process' network track appears again.
Note that I selected profiler.firefox.com origin in the top left corner in the tab selector. I think this is happening because the indexes for the tab selector are not translated (probably tabToThreadIndexesMap).
Previously, hidden non-thread tracks (screenshots, network, IPC) reappeared after publishing a sanitized profile when sanitization also removed a thread: the URL state's hidden-track sets and track-order were reset whenever threads were re-indexed, even though the non-thread tracks still existed in the sanitized profile. attemptToPublish now builds an old to new TrackIndex map by matching tracks on stable identity (pid, screenshot id, threadIndex, counterIndex, visual-progress singleton) and translates hiddenGlobalTracks, globalTrackOrder, hiddenLocalTracksByPid, and localTrackOrderByPid through it on SANITIZED_PROFILE_PUBLISHED. Marker tracks key on a string-table index that sanitization reshuffles, so they are not matched. Fixes firefox-devtools#2947
When a tab filter was active during publishing, a hidden non-thread track (e.g. a screenshot or a per-process network track) could reappear after sanitization. attemptToPublish was remapping the URL state's hidden-track set against a global track list filtered using the prePublishedState's tabToThreadIndexesMap; that map's Set<ThreadIndex> values referred to old thread indexes, so when sanitization shifted surviving threads down by removing an earlier thread, filterGlobalTracksByTab picked the wrong threads and the hidden track ended up at a stale TrackIndex. attemptToPublish now builds the map from the sanitized profile's threads and pages so its ThreadIndex values are valid in the new track-index space. The innerWindowID to tabID computation is extracted into a shared computeInnerWindowIDToTabMap helper that the getInnerWindowIDToTabMap selector also uses.
Thanks! Fixed now. |
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Thanks for the fix! I think there is still a bug where _trackIdentityKey returns null for the marker tracks. Is there a reason why it's returning null? I couldn't find an example profile that I could reproduce the bug, but I suspect that it will be an issue for some profiles.
Thank you for the review, @canova! You're right that this is still a real gap. The fix could be to translate |
A marker LocalTrack identifies by (threadIndex, markerSchema, markerName) where markerName is an IndexIntoStringTable. Sanitization rebuilds the string table via computeCompactedProfile, and the previous remap helper skipped marker tracks entirely; hidden marker tracks then reset to default visibility after publish. _trackIdentityKey now takes an oldStringToNewStringPlusOne Int32Array (already in ProfileIndexTranslationMaps) and keys marker tracks on (translated threadIndex, markerSchema.name, translated markerName). attemptToPublish passes the map through both global and local computeOldTrackIndexToNewTrackIndexMap calls. Also move computeOldCounterIndexToNew from actions/publish.ts to profile-logic/profile-data.ts, alongside the other compute* helpers, since it is pure computation rather than an action.
Changes: [fatadel] Remove unused dependencies from package.json (#6010) [Nazım Can Altınova] Make profiler-cli work in sandboxed environments (#6003) [Markus Stange] Make profiler-edit run profile compacting before writing out the file (#6015) [Markus Stange] Migrate from prettier to oxfmt (#5986) [Markus Stange] Add a --symbolicate-wasm arg to profiler-edit. (#6008) [Markus Stange] Build and upload the cli artifact in PRs (#6020) [Markus Stange] Use @streamparser/json if the input is too large to fit in a V8 string (#6016) [Nazım Can Altınova] Print also the status output right after cli `load` command (#6019) [Nicolas Chevobbe] Update devtools-reps to 0.27.7 (#6030) [Nazım Can Altınova] Include `--search` option in `pq filter push` (#6026) [Nazım Can Altınova] Update all Yarn dependencies (2026-05-20) (#6033) [fatadel] Translate URL track-index state through profile sanitization (#6000) [Markus Stange] Make withSize use a wrapper element so that it can stop calling findDOMNode (#5988) [Markus Stange] Fix dhat importer (#6036) [Nazım Can Altınova] Annotate inlined frames in CLI call trees and stacks (#6041) [Nazım Can Altınova] Use proper types in cli tests instead of custom inline types (#6038) [Nazım Can Altınova] Fix text truncation for frames named after Object.prototype methods (#6044) [Nazım Can Altınova] Add missing key props to CodeErrorOverlay error list items (#6047) [depfu[bot]] ⬆️ Update oxfmt to version 0.51.0 (#6054) [Nazım Can Altınova] 🔃 Sync: l10n -> main (May 26, 2026) (#6058) [Nazım Can Altınova] Use URL-state symbol server for `profiler-cli function annotate` (#6051) [Nazım Can Altınova] Bump profiler-cli version to 0.2.0 (#6059) And special thanks to our localizers: fr: YD sr: Марко Костић (Marko Kostić) tr: Ali Demirtaş zh-CN: Olvcpr423 zh-CN: wxie
2 participants
Previously, hidden non-thread tracks (screenshots, network, IPC, marker) reappeared after publishing a sanitized profile when sanitization also removed a thread: the URL state's hidden-track sets and track-order were reset whenever threads were re-indexed, even though the non-thread tracks still existed in the sanitized profile. When a tab filter was active, the bug also affected tracks in surviving processes because the
tabToThreadIndexesMapused to filter the new global track list still referenced pre-sanitization thread indexes.attemptToPublishnow builds an old-to-newTrackIndexmap by matching tracks on stable identity (pid, screenshot id, threadIndex, counterIndex, visual-progress singleton, or marker schema name plus marker name string) and translateshiddenGlobalTracks,globalTrackOrder,hiddenLocalTracksByPid, andlocalTrackOrderByPidthrough it onSANITIZED_PROFILE_PUBLISHED. Old-side thread, counter, and string table indexes are normalized through the maps inProfileIndexTranslationMapsbefore comparison. ThetabToThreadIndexesMapis rebuilt from the sanitized profile's threads and pages so itsThreadIndexvalues are valid in the new track-index space; theinnerWindowID → tabIDcomputation is extracted into a sharedcomputeInnerWindowIDToTabMaphelper that the existinggetInnerWindowIDToTabMapselector also uses.Fixes #2947.
The work is built upon the draft PR by @julienw.
STR are available in the original issues, here is the deploy preview profile.